Skip to content

feat: vcs: support Jujutsu as a diff-provider - #12022

Draft
poliorcetics wants to merge 4 commits into
helix-editor:masterfrom
poliorcetics:ab/push-rlymwpyuzrrw
Draft

feat: vcs: support Jujutsu as a diff-provider#12022
poliorcetics wants to merge 4 commits into
helix-editor:masterfrom
poliorcetics:ab/push-rlymwpyuzrrw

Conversation

@poliorcetics

@poliorcetics poliorcetics commented Nov 7, 2024

Copy link
Copy Markdown
Contributor

Built on top of #9951, I'll be waiting for it to be merged to un-draft this


Jujutsu (jj) is a new change-based VCS (whereas git is branch-based).

In this PR, I add the ability for helix to get the diffs and current head, behind a feature called jj that is active by default. That would make it the first editor I know off that officially handles that VCS!

To handle all current, future and private backends (Google already has one I believe), I instead made it so Helix can behave as a diff tool for Jujutsu and then use that as a subcommand to get the diff base.

For the head, I simply used the templating system to extract all relevant informations.

Jujutsu has a library, called jj-lib but it's not ready for use in third party programs and wouldn't fix the issue of custom backends anyway.

Testing

Since testing needs jj installed, I haven't written any for Helix yet to discuss how to do it:

  1. Ignore the issue. I don't like it much, but it doesn't cost CI time and doesn't need any change. For a feature that probably won't be used much yet, that's probably ok for a time
  2. Install jj in CI and use it to test the new feature

@the-mikedavis the-mikedavis added the A-vcs Area: Version control system interaction label Nov 7, 2024
@poliorcetics
poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch 2 times, most recently from d07b9ff to 601e3a5 Compare November 14, 2024 20:46
@poliorcetics
poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch 2 times, most recently from 422e761 to fc7c8d1 Compare November 27, 2024 19:09
@yerlaser

Copy link
Copy Markdown
Contributor

Hi @poliorcetics,

Thanks for this PR, hope it will be merged soon.
Quick question. I just built from it but all I get the change hash in the status line but no diff indicators in the gutter.
Did I miss something?

@VasanthakumarV

Copy link
Copy Markdown

Hi @poliorcetics,

Thank you for working on integrating jj into helix, I am using helix from your branch, and it has been great so far.

I am new to jj, and this PR is still a draft, but wanted to make a suggestion on the status-line entry,

Will description.first_line() be more useful thanchange_id in the status-line message?

Change-Id:
image

Description:
image

@poliorcetics

Copy link
Copy Markdown
Contributor Author

I am new to jj, and this PR is still a draft, but wanted to make a suggestion on the status-line entry,

Will description.first_line() be more useful thanchange_id in the status-line message?

It could easily be done code-wise, but I'm trying to get the same behavior as the git backend (to make it easier for this PR to get in), so I don't think I'll do it

If maintainers confirm it would be ok to add, I'll do it :)

Comment thread helix-vcs/src/jj.rs Outdated
@bryceberger

bryceberger commented Jan 5, 2025

Copy link
Copy Markdown
Contributor

This gets a bit weird with reloading newly-added or untracked files.

  1. Create a new file, add some content. Nothing shows up in the gutter (expected).
  2. Reload the file with :reload. Still nothing in gutter (expected).
  3. Edit the file. Edits show up as changes (unexpected!!)

Each time the file is reloaded, the diff base is set to the current file.

I have somewhat of a fix for this at d4a0e44. If the file is untracked, return early. If the file is new, return an empty base. Otherwise, continue as before.

I'm not really a fan of my implementation, since it relies on jj emitting a warning message if the file isn't found in the repo. Could do without that by doing a separate (jj file list).contains(file_relative_to_root), but I didn't want to add another jj invocation.

@bryceberger

Copy link
Copy Markdown
Contributor

Was curious to see what an implementation using jj-lib would look like. Have a proof of concept at https://github.com/bryceberger/helix/tree/jj-lib (permalink: a8a105d). Needs better integration with helix's async code, doesn't solve custom backends. Otherwise seems to work.

@poliorcetics
poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch 2 times, most recently from da5d835 to 035ccd0 Compare February 2, 2025 13:23
@poliorcetics
poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch from 035ccd0 to b58879e Compare March 29, 2025 12:02
@poliorcetics
poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch from b58879e to 8b10cc2 Compare April 12, 2025 17:44
@poliorcetics
poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch from 8b10cc2 to 5e0f205 Compare May 1, 2025 10:58
@poliorcetics
poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch from 5e0f205 to 30dd7e2 Compare May 14, 2025 16:59
@poliorcetics
poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch from 30dd7e2 to ec98a74 Compare June 16, 2025 21:30
@yerlaser

yerlaser commented Jul 1, 2025

Copy link
Copy Markdown
Contributor

Looks like this feature doesn't always work.
Often it fails to show any diffs.

I experienced this on a non-collocated repo as well as (albeit less often) on pure Git repos.

@poliorcetics
poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch from ec98a74 to ee0e561 Compare July 19, 2025 15:13
@poliorcetics
poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch from ee0e561 to f597775 Compare August 5, 2025 21:28
@icorbrey

icorbrey commented Aug 7, 2025

Copy link
Copy Markdown
Contributor

I'm not getting any gutter information in a pure JJ repo, and there aren't any logs to suggest something went wrong. Does this PR not provide gutter information? If not could we? I'd be happy to hack on this and see if I can get it going

@yerlaser

yerlaser commented Aug 7, 2025

Copy link
Copy Markdown
Contributor

I'm not getting any gutter information in a pure JJ repo, and there aren't any logs to suggest something went wrong. Does this PR not provide gutter information? If not could we? I'd be happy to hack on this and see if I can get it going

It worked intermittently for me as well.

I suspect it hits some threshold timeout in case the repo is large enough.
You can try to init a blank repo and try it there to check if it's the same issue as I faced.

@poliorcetics
poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch 3 times, most recently from 8ccbcdc to 6e20c16 Compare April 15, 2026 22:10
Comment thread helix-vcs/src/jj.rs Outdated
@poliorcetics
poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch from 6e20c16 to 7798fd4 Compare May 20, 2026 17:05
@poliorcetics
poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch from 7798fd4 to 061d666 Compare May 30, 2026 13:24
@glehmann

glehmann commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

A small suggestion, to avoid duplicated entries in the change picker in case of conflict:

diff --git a/helix-vcs/src/jj.rs b/helix-vcs/src/jj.rs
index ab0799cdc1..711652dade 100644
--- a/helix-vcs/src/jj.rs
+++ b/helix-vcs/src/jj.rs
@@ -5,6 +5,7 @@
 //! Instead in case there *is* a diff to base ourselves on, we copy it to a tempfile or just use the
 //! current file if not.
 
+use std::collections::HashSet;
 use std::path::{Path, PathBuf};
 use std::process::Command;
 use std::sync::Arc;
@@ -178,12 +179,14 @@
 
     anyhow::ensure!(out.status.success(), "`jj file list` executed but failed");
 
+    let mut conflicted = HashSet::new();
     for entry in split_double_slash(&out.stdout, true) {
         if entry.is_empty() {
             continue;
         }
 
         let path = make_pathbuf(entry);
+        conflicted.insert(path.clone());
 
         if !callback(Ok(FileChange::Conflict { path })) {
             return Ok(());
@@ -257,6 +260,11 @@
             continue;
         };
 
+        // Skip files already reported as conflicted above
+        if conflicted.contains(change.path()) {
+            continue;
+        }
+
         if !callback(Ok(change)) {
             return Ok(());
         }

@poliorcetics

Copy link
Copy Markdown
Contributor Author

Recent changes:

  • Use the relatively new --no-integrate-operation, which is nicer than --ignore-working-copy because it will immediately find diffs (and everything else) without having to run a jj command outside of Helix, but still without modifying the underlying JJ repo state
  • changed_file_picker: made faster by running only one jj command for it
  • changed_file_picker: deduplicated conflicted files that have also been modified, directly via the JJ template
  • current commit & branch: slightly smarter revset that should be faster to evaluate (pretty much imperceptible except in humongous repositories)

@poliorcetics
poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch 3 times, most recently from 900399d to af215b3 Compare July 25, 2026 09:30
Clippy says it's currently 784 bytes. Given it's intended to be long lived,
allocating to fix that is not an issue.
@poliorcetics
poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch from af215b3 to 7936b95 Compare July 25, 2026 09:44
@poliorcetics
poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch 2 times, most recently from b99c20b to ddfb07e Compare August 3, 2026 15:01
@poliorcetics

poliorcetics commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

EDIT: oups, there is a bug in that, currently fixing done 🎉

Previously if you reloaded a newly added file, the gutter diff would start being computed from the file on disk, that is not mark the entire file as "added" but sections as "modified" in the gutter.

This is now fixed :)

@poliorcetics
poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch 2 times, most recently from e3fb7dc to 397e595 Compare August 10, 2026 13:06
@poliorcetics

Copy link
Copy Markdown
Contributor Author

Fixed a bug with JJ 0.44.0


 error: test failed, to rerun pass `-p helix-term --test integration`

Caused by:
  process didn't exit successfully: `/home/runner/work/helix/helix/target/integration/deps/integration-d67ba11fdaf58aad` (signal: 6, SIGABRT: process abort signal)

This is not the first failure I see like this in Github actions, I saw it in other repos too, I don't think this is something I can fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-vcs Area: Version control system interaction

Projects

None yet

Development

Successfully merging this pull request may close these issues.